os/board/bk7239n/: Change memory access permission in MPU by namanjain7 · Pull Request #7290 · Samsung/TizenRT

namanjain7 · 2026-04-20T05:14:46Z

Memory region is accessible only in priviledge mode. Unprivileged code cannot access it at all. This commit protects critical system memory (kernel region) from app memory.

ewoodev

firstly, BSP MPU setting is only PRIV, this loading logic is allow UNPRIV

ewoodev · 2026-04-20T06:49:08Z

Could you please it also?

ewoodev · 2026-04-20T06:49:24Z

please check it also

ewoodev · 2026-04-20T06:49:41Z

please check it also

Hello @namanjain7 @ewoodev

During internal testing, we found that the Bluetooth app directly uses kernel-space memory. The call path is as follows:

When the BLE server disconnects, ble_server_connected_cb is invoked; with the current Bluetooth architecture, this callback is called directly from the kernel layer.

static ble_server_init_config server_config = {
ble_server_connected_cb,
ble_server_disconnected_cb,
ble_server_mtu_update_cb,
ble_server_passkey_display_cb,
ble_server_pair_bond_cb,
true,
gatt_profile, sizeof(gatt_profile) / sizeof(ble_server_gatt_t)
};

The callback invokes the ble_server_set_adv_data interface. Inside that interface, a local variable is defined: blemgr_msg_s msg = {BLE_CMD_SET_ADV_DATA, BLE_MANAGER_FAIL, (void *)(data), NULL};. That local variable lives in kernel space, yet it is consumed in the BLE message handler task (which belongs to app space). The usage site is:

ble_result_e blemgr_handle_request(blemgr_msg_s *msg) {
trble_result_e ret = TRBLE_FAIL;
blemgr_msg_params queue_msg = {
.evt = msg->event,
.count = 3
};

So we configured the system to allow the app to access kernel space.

Currently, the structure is not such that BLE requests are directly passed from within the callback called from the Kernel thread.
In the file named ble_response_handler.c, server and client callbacks are wrapped once, so the callback of the application layer is designed to be called by the user thread. (ecode only)
The structure is as follows:
Kernel thread calls response handler callback -> mq enqueue -> user thread mq dequeue -> calls application layer callback
Given this structure, it seems there would be no issues if kernel access is blocked on the user side.

Hello @giwon-nam The issue we tested is based on the code on GitHub, and tests related to ble_rmc will have this problem. Can you help me double-check it?

It seems that the issue occurred in the part where the connected callback is called during disconnection on the TP1x Plus.
I will contact Beken and request that during disconnection, only the disconnected callback should be called, and not the connected callback.

Hi @giwon-nam ,
In the ble_perfs and ble_tester examples, the attribute write callback ble_peri_cb_charact_rmc_sync invokes ble_server_attr_get_data. So this appears to cause the same server disconnection issue, What is the recommended way to avoid this?

This state is not safetly..

we didn't receive any report issue about it.
you have to open issue this.

we will apply this PR. Please reslove the ble issue ASAP.

Hello, @lingzhou2018
For now, the ble_server_attr_get_data function you mentioned appears to be functioning without issues.
Currently, on the ecode side, ble_server_attr_get_data is called within the attribute callback invoked by the kernel thread to check what data was written.
It has been confirmed that there are no issues on the ecode side after reflecting the content of the current PR.
If the schedule is urgent, it would be better to merge this PR first and then further analyze why crashes occur in ble_perfs and ble_tester.

seokhun-eom24 · 2026-04-20T17:44:57Z

Automated nightly Codex PR review
PR: #7290 — os/board/bk7239n/: Change memory access permission in MPU
PR URL: #7290
GitHub updatedAt: 2026-04-22T10:06:26Z
Refreshed (UTC): 2026-04-22 17:47:58 UTC
Refreshed (KST): 2026-04-23 02:47:58 KST
Comment model: single evolving Codex-authored PR comment

PR #7290 — os/board/bk7239n/: Change memory access permission in MPU

This review done by codex. AI reviews can be inaccurate.
UTC: 2026-04-22 17:46 UTC
KST (UTC+9): 2026-04-23 02:46 KST

Repository: Samsung/TizenRT

Base → Head: master → pr/7290

HEAD Commit: fccee8d9546f4b38b4c343765b897ac8d596149f

Scope: Static review of the bk7239n protected-build MPU permission tightening in os/board/bk7239n/src/middleware/soc/bk7239n/mpu_cfg_s.c; no build, boot, or runtime fault-injection validation was run in this review.

🔎 Review Summary

Category	Status
Build / Compile Status	⚠️ Not run
Functional Correctness	✅
Validation Coverage	⚠️

Final Verdict: 💬 Comment

🚨 Must-Fix Issues

No blocking issues found in static review.

🟡 Nice-to-Have Improvements

1. Add one protected-build smoke pass for the new MPU policy

Item	Details
Location	`os/board/bk7239n/src/middleware/soc/bk7239n/mpu_cfg_s.c:105`
Severity	Medium
Type	Validation

Problem
The permission tightening itself looks internally consistent, but this review only confirms it by static inspection. The changed regions are active in the protected CONFIG_SPE=0 bk7239n builds (build/configs/bk7239n/loadable_all/defconfig:23-31, build/configs/bk7239n/loadable_all/defconfig:232-235) and rely on runtime overlap/priority behavior with later app/common MPU regions (os/binfmt/binfmt_arch_apis.c:33-72, os/arch/arm/src/common/up_restoretask.c:92-99).

Impact

A regression here would show up as an early MemManage fault rather than a compile-time error.
The current diff changes kernel SRAM/PSRAM/MMIO access policy, so the highest-value risk is boot-time or first-app-start failure under protected/loadable configurations.

Recommended Action

Build and boot at least bk7239n/loadable_all and bk7239n/loadable_apps.
Start the common binary and one user app, then confirm the app still runs with its dedicated heap region from CONFIG_HEAP_INDEX_LOADED_APP=2.
Add one negative test that attempts direct user-mode access to a now-kernel-only window and confirms the expected MPU fault or denial.

Expected Output

Area	Example Target / Check	Result
Protected boot	`bk7239n/loadable_all` boot to shell	Not Run
Common/app startup	Load common + one user app after boot	Not Run
Access control	User-mode probe against `0x38000000`/kernel PSRAM window faults as expected	Not Run

✅ Notable Improvements

Notable Improvements

✔ Kernel-only windows are now aligned with the protected-loadable layout

The changed PSRAM/kernel regions line up with the kernel heap partitions, while loaded apps allocate from heap index 2 instead (build/configs/bk7239n/loadable_all/defconfig:232-235, os/binfmt/binfmt_arch_apis.c:89-103).
The first PSRAM kernel window ends exactly at 0x70500000, which is where the app heap region begins, so the privilege tightening does not obviously consume the app heap range (build/configs/bk7239n/scripts/bk7239n_bsp_loadable_all.ld:507-515).

✔ The patch removes unnecessary executable permissions from data-only regions

Shared SRAM at 0x38000000 and PSRAM data at __psram_data_start__ are data windows, and the new MPU_EXEC_NEVER setting matches that role (os/board/bk7239n/src/middleware/soc/bk7239n/mpu_cfg_s.c:107-122, os/board/bk7239n/src/middleware/soc/bk7239n/mpu_cfg_s.c:141-150).
Tightening those ranges avoids leaving RWX-style mappings on memory that the linker scripts describe as SRAM/PSRAM data rather than code (os/board/bk7239n/src/middleware/soc/bk7239n/bk7239_bsp.ld:44-48).

✔ The diff matches the way user binaries receive access in protected mode

User/common binaries get their own later MPU regions from the binfmt path instead of inheriting board-specific access to all kernel heaps (os/binfmt/binfmt_arch_apis.c:33-72).
On context switch, those per-binary regions are restored for non-kernel threads, which is the right place for app visibility rather than keeping board-wide regions unprivileged (os/arch/arm/src/common/up_restoretask.c:92-99).

🧾 Final Assessment

Must-Fix Summary

No blocking MPU issue is evident from the changed code paths.

Nice-to-Have Summary

This should still get one protected-build boot smoke and one negative access-control probe before merge so the new policy is grounded in runtime evidence.

Residual Risk / Test Gap

Static review only; no bk7239n/loadable_all or bk7239n/loadable_apps build was run.
No runtime confirmation was collected for expected user-mode faults on the newly privileged-only regions.

📌 Final Verdict

💬 Comment

No blocking defect is apparent in the patch itself: the tightened regions match kernel-owned SRAM/PSRAM/MMIO ranges, and user binaries appear to keep their own later MPU mappings. The remaining gap is verification, not design, so I would add one protected-build smoke run before calling this fully de-risked.

amandeep-samsung

MPU PRIV is applied to all defined 7 MPU regions.

ewoodev · 2026-04-21T11:56:31Z

@@ -144,7 +144,7 @@ void bk_mpu_init(void)
            mpu_cfg.region_base = psram_data_base;
            mpu_cfg.region_size = (psram_data_limit - psram_data_base) - 32;
            mpu_cfg.xn = MPU_EXEC_ALLOW;


Should this bss and data be protected about EXEC?

No, it should be MPU_EXEC_NEVER.
we will change and verify it

ewoodev · 2026-04-21T11:57:07Z

@@ -176,7 +176,7 @@ void bk_mpu_init(void)
            mpu_cfg.region_base = psram_heap_base;
            mpu_cfg.region_size = (psram_heap_limit - psram_heap_base) - 32;
            mpu_cfg.xn = MPU_EXEC_NEVER;


heap also..

mpu_cfg.xn = MPU_EXEC_NEVER; ok

Memory region is accessible only in priviledge mode. Unprivileged code cannot access it at all. This commit protects critical system memory (kernel region) from app memory.

amandeep-samsung

new changes for MPU_EXEC_ALLOW to MPU_EXEC_NEVER are verified on bk7239n board for

data + bss region and
shared memory region

ewoodev reviewed Apr 20, 2026

View reviewed changes

namanjain7 force-pushed the mpu_memory_changes branch from bd6ecbb to cec81a8 Compare April 21, 2026 09:10

amandeep-samsung approved these changes Apr 21, 2026

View reviewed changes

ewoodev reviewed Apr 22, 2026

View reviewed changes

giwon-nam mentioned this pull request Apr 22, 2026

os/board/bk7239n:Call only disconnected callback when BLE server disconnected #7295

Merged

os/board/bk7239n/: Change memory access permission in MPU

fccee8d

Memory region is accessible only in priviledge mode. Unprivileged code cannot access it at all. This commit protects critical system memory (kernel region) from app memory.

namanjain7 force-pushed the mpu_memory_changes branch from cec81a8 to fccee8d Compare April 22, 2026 08:31

amandeep-samsung reviewed Apr 22, 2026

View reviewed changes

Conversation

namanjain7 commented Apr 20, 2026

Uh oh!

ewoodev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lingzhou2018 Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seokhun-eom24 commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR #7290 — os/board/bk7239n/: Change memory access permission in MPU

🔎 Review Summary

🚨 Must-Fix Issues

🟡 Nice-to-Have Improvements

✅ Notable Improvements

✔ Kernel-only windows are now aligned with the protected-loadable layout

✔ The patch removes unnecessary executable permissions from data-only regions

✔ The diff matches the way user binaries receive access in protected mode

🧾 Final Assessment

Must-Fix Summary

Nice-to-Have Summary

Residual Risk / Test Gap

📌 Final Verdict

Uh oh!

amandeep-samsung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amandeep-samsung left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lingzhou2018 Apr 21, 2026 •

edited

Loading

seokhun-eom24 commented Apr 20, 2026 •

edited

Loading